Skip to content

fix(cli): build \os dev\'s MCP connect hint from the resolved origin, not the listen socket - #16812

Merged
os-project-manager merged 2 commits into
mainfrom
claude/issue-16734-mcp-connect-hint-origin
Sep 8, 2026
Merged

fix(cli): build \os dev\'s MCP connect hint from the resolved origin, not the listen socket#16812
os-project-manager merged 2 commits into
mainfrom
claude/issue-16734-mcp-connect-hint-origin

Conversation

@claude

@claude claude Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Fixes #16734

Clause-②: no

os dev printed two MCP addresses in one boot output, from two origin resolvers, and only one of them knew the canonical origin. The ready banner's ➜ MCP: row (printServerReady, packages/cli/src/utils/format.ts) goes through resolveAuthBaseUrlOS_AUTH_URL → legacy BETTER_AUTH_URLOS_BASE_URL → the http://localhost:PORT tail. The 🤖 MCP server — connect a coding agent block in packages/cli/src/commands/dev.ts built its base from the serve child's objectstack:listening url, which is the socket the child BOUND, by construction. OS_AUTH_URL never entered that expression.

That block's Connect line is a command the reader pastes, so the wrong origin is not cosmetic: claude mcp add registers an entry against an address discovery never advertises and, behind a TLS proxy, nothing can reach — and the two rows disagreeing made the correct one look like the typo.

What changed

  • packages/cli/src/commands/dev.ts — the block moves into an exported printMcpConnectHint({ boundPort, name }) that resolves its origin through resolveAuthBaseUrl, the same function serve's banner call site uses, with the port the child ACTUALLY bound. The precedence chain is not restated anywhere in dev.
  • The call site inside the objectstack:listening handler now passes boundPort: actual and nothing else; the base expression built from msg.url is gone.
  • When the chain yields no parseable origin (baseOrigin === null — a set-but-empty OS_AUTH_URL=, or a value with no scheme) the block is omitted. The banner's documented rule for that case is "print the paths with no origin and name the variable that fixes it, never guess"; a claude mcp add line has no paths-only form, so the honest output is no block rather than reprinting, on the same screen, the exact address the banner just refused to print.
  • packages/cli/src/commands/dev-mcp-connect-hint-origin.test.ts — new, 11 cases.
  • .changeset/dev-mcp-connect-hint-origin.md@objectstack/cli: patch.

packages/cli/src/utils/format.ts is untouched: it is the reference implementation, and nothing in it needed to change. The Endpoint / Skill / Connect wording is unchanged.

start.ts already carries the rule this PR applies to dev, in its own words: "⛔ Do not reintroduce an address here fed from the child's ipc objectstack:listening message". dev was the one command still doing it.

Anchors, re-derived on the current base

The card's line numbers were written against 5e53d73d; base here is dd2184ae25.

card found on dd2184ae25 rot
dev.ts:504-505 — the base expression dev.ts:500-501 4 lines
dev.ts:503-508 — the printed block dev.ts:503-508 0 lines
format.ts:816 — the ➜ MCP: row format.ts:816 0 lines

验收备注

1. With OS_AUTH_URL set, all three block lines and the ➜ MCP: row agree

Asserted on both rows of one captured boot output, never on the block alone: each case installs one buffer over console.error (the banner) and console.log (the hint), in call order, then drives both printers under one environment and one bound port. The card's own reason — "Two printers that disagree cannot be pinned by reading one of them."

The measured repro is a case verbatim: dev -p 4001 under OS_AUTH_URL=https://localhost:4443 asserts the banner row, Endpoint, Skill and Connect all carry https://localhost:4443, that localhost:4001 appears nowhere in the capture, and — independent of which rows were named — that the set of distinct MCP origins in the whole capture is exactly one. Two further cases carry the same assertion onto a public origin (with and without a port) and onto the rest of the chain (BETTER_AUTH_URL, then OS_BASE_URL).

2. With OS_AUTH_URL unset, the block still prints the LISTEN origin

This falls out of the resolver rather than from a second fallback: with none of the three variables set, resolveAuthBaseUrl's own built-in tail answers http://localhost:PORT for the port it is handed, and the hint is handed the port the child actually bound.

Three cases: an ordinary local boot on 3000; the auto-shifted port (3000 busy, bound 3001) which additionally asserts 3000 appears nowhere in the capture; and an ephemeral bound port (45064), the --port 0 shape.

That control is measured, not asserted — ablation leg 2 below is exactly the "reach for a canonical origin and ignore the bound port" fix, and these are the cases that catch it.

3. The precedence chain is reused, not reimplemented; the existing pins are unchanged and still green

resolveAuthBaseUrl and formatUnusableAuthBaseUrlDiagnostic are untouched — the diff adds an import of the first and changes no line of serve.ts. The set-but-empty behaviour keeps its existing owner; what this PR adds is a pin that both printers obey it (the banner prints paths only, the hint prints nothing, and never-consulted.example.com from a set OS_BASE_URL appears in neither).

packages/cli/src/commands/serve-auth-base-url-diagnostic.test.ts25 passed, unchanged file. Alongside it, serve-banner-external-base-url.test.ts 6 passed, serve-bound-port-publication.test.ts 21 passed, test/vitest-tiers-partition.test.ts 22 passed, and the new file 11 passed — 85 of 85 in one run at b466fc5d90.

Reverse verification — two legs, both restored byte-identical

Committed first, mutated on disk, proven on disk (occurrence counts for the removed text and the injected marker, plus git hash-object), run, then restored with git checkout HEAD -- and re-proven (git diff HEAD empty, blob equal to the HEAD blob). Both legs ran with a trap ... EXIT INT TERM restore.

leg mutation blob result
1 — the defect, restored origin taken from the listen socket again 8bb27f19dba2980ca600 5 failed / 6 passed — the three OS_AUTH_URL-set cases and both unusable-value cases red; the three unset cases stayed green, as they must
2 — the port ignored resolveAuthBaseUrl(3000) instead of the bound port 8bb27f19db56d7bfd6e4 2 failed / 9 passed — only the shifted-port and ephemeral-port cases red; acceptance 1 stayed green

Restore both times: blob back to 8bb27f19db, git diff HEAD empty. Leg 1 shows the pin can see the reported defect; leg 2 shows acceptance 2 is a real control and not decoration. No permanent test file carries either mutation.

Resolution note: both printers and the test resolve ./dev.js / ./serve.js to source inside packages/cli (the unit project transforms TS directly), so neither leg is dist-mediated and no rebuild sits between the mutation and the reading.

Checks run

All at b466fc5d90 unless noted. Exit codes captured before any pipe.

  • pnpm --filter '@objectstack/cli^...' build — exit 0 (the dependency closure).
  • pnpm --filter @objectstack/cli exec vitest run --project unit186 files, 2555 passed + 6 expected-fail, exit 0. First attempt reported 2 suites failing to collect with packages/cli is not built (./dist/index.js is absent) — a stated prerequisite, not a finding; both green after pnpm --filter @objectstack/cli build and in the full re-run above.
  • pnpm --filter @objectstack/cli typecheck — exit 0 (tsc --noEmit plus check:test-typecheck, the test-layer debt ledger held).
  • pnpm lint (eslint . --no-inline-config, the whole repo, no narrowing) — exit 0, 1m22s.
  • Gate families derived from the real change set, not from a hand-written list: node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack, then reconciled with --ran: 57 derived, 57 run, 0 NOT-MEASURED, 0 UNRUN. 54 exit 0. The other three are PREREQUISITE NOT MET (exit 3, the gates' own distinct code) and say nothing in either direction — each needs a full workspace build, which is CI's run: check:dual-build-cjs-loads, check:i18n-coverage, check:type-check-debt. The first half of that last one, check:type-check-coverage, ran and passed.
  • The five gates the derivation flags as "roster under a directory one of your paths is in — the silence is not evidence in either direction" were run rather than read: check-changeset-fixed, check:authz-resolver, check:error-code-casing, check:filter-alias-parity, check:scaffold-emission-policy — all exit 0.
  • check:dts-closure needed a second look: exit 0 during the sweep, exit 1 after the unit tier had run, naming 7 packages with JS-only dist/ and no declarations — none of them in this diff, none of them downstream of it. Rebuilding exactly those 7 restored it to 63 packages swept, 155/155 declarations present, exit 0. Container tree state, not this change.
  • pnpm check:nul-bytes exit 0, plus a direct control-character scan over both edited files (grep -naP) — no hits.
  • Derivation staleness, declared: the tool warned STALE TREE against origin/main 78bc4ad587, naming .github/workflows/publish-smoke.yml and scripts/pm/check-skill-line-ratchet.mjs. Re-deriving after git fetch origin main produced a byte-identical command list. Neither can add a family for these paths — publish-smoke is workflow_run/schedule-driven with no PR path filter, and the skill ratchet's population is skills/**, which this diff does not touch.

Tier placement

Both tiers are derived by packages/cli/vitest-tiers.ts, and the new file lands in unit (it spawns nothing and boots no kernel). The integration tier's include list names no file this diff touches, so the integration tier is declared to CI. test/vitest-tiers-partition.test.ts was run and is green.

Docs drift advisory — answered, 0 pages edited

The bot's Docs Drift Check on this PR truncated its list at 15 rows, so it was re-derived locally rather than read off the comment:

node scripts/docs-audit/affected-docs.mjs --json 56ee7aef1d49d7583e0cb268dc6cbd1c34ee8fd0

34 docs, 4 anchors, 1 changed package. Provenance differs from the bot's on purpose and both readings are kept: the bot computed on merge commit 8652f200e599 and said outright that "That checkout carried uncommitted changes, so the commit above does not fully identify what was read"; this run reports head b466fc5d90ec, diffBase dd2184ae25, dirty: false, same sinceRef. The anchors are printMcpConnectHint (symbol), os dev (command), /api/v1/mcp and /api/v1/mcp/skill (routes).

The 4 release-owned pages — content/docs/releases/ v13, v14, v16, v17 — were not edited and are not this PR's to edit. Release notes are written centrally at release time; this PR's only input to them is its changeset. For the record, the two rows that actually describe this block (v16.mdx lines 75 and 468 — "the os dev banner prints the MCP endpoint, the agent-skill URL, and a ready-to-paste claude mcp add command") remain true after this change: all three lines still print, they simply carry the reachable origin now. Nothing to correct there even in a dedicated docs PR.

The other 30: verified unaffected, grouped by why. Every one of them was listed through a generic anchor, not through anything this diff altered:

  • listed via os dev (the command name, which many pages mention in passing) — no flag, no default, no output contract of os dev changed;
  • listed via /api/v1/mcp or /api/v1/mcp/skill — both are path literals this diff moves between two files and does not change; no route moved, no route was added or removed;
  • listed via printMcpConnectHint — a symbol introduced by this PR, so no page can be naming it.

30 rows is what @objectstack/cli plus the MCP route path always produces; the count is not evidence of breakage, and none of the 30 was edited.

Hand-read for the bot's declared blind spot — a page stating the rule by its INPUTS while the diff changed the EMITTER. This change does carry a rule (the connect hint is built from the resolved origin, not the listen socket), so content/ was grepped by hand for that rule's own vocabulary, whether or not the bot listed the page:

  • grep -rn "objectstack:listening|listen origin|bound socket" content/zero hits. No page anywhere states which origin the hint is built from, or mentions the listen socket or the two-row disagreement. There is no page for this rule to falsify.
  • content/docs/ai/connect-mcp.mdx and content/docs/getting-started/build-with-claude-code.mdx are the two pages that print a claude mcp add line: http://localhost:3000/api/v1/mcp labelled "local dev server" and https://your-deployment.example.com/api/v1/mcp labelled "a deployed instance". That is exactly what the fixed hint now emits in each case — the first is the resolver's unset tail, the second is what OS_AUTH_URL resolves to. The change moves the code toward these pages; before it, only the first form could ever be printed. Verified unaffected, unedited.
  • content/docs/deployment/cli.mdx:45"The boot banner also prints the app's MCP endpoint (/api/v1/mcp)". It names the PATH and no origin, and it is about the ready banner in format.ts, which this diff does not touch. Still true.
  • The one behaviour change that could falsify a promise is the block being omitted when no origin resolves. No page promises that block; the sentence above is about the banner's MCP: row, which is unconditional and unchanged.

Outcome: 0 of 34 pages needed a change, 0 were edited, and this stays a 3-file PR.

Out of scope, noted and not filed

  • ListeningMessage.url now has no reader in packages/cli/src: dev was its last consumer, and start declines it on purpose. The field is still published and still pinned by serve-bound-port-publication.test.ts, and the same url is written to the runtime state file where it is the right value. Dead-ish code, not a defect. Carrier: the next PR touching publishBoundPort / ListeningMessage.
  • No reproducible defect, contract violation, or metadata-authoring trap was found outside this card's scope, so nothing was filed.

🤖 Generated with Claude Code

https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8


Generated by Claude Code

… not the listen socket

`os dev` printed two MCP addresses in one boot output. The ready banner's
`MCP:` row goes through `resolveAuthBaseUrl` (`OS_AUTH_URL` -> legacy
`BETTER_AUTH_URL` -> `OS_BASE_URL` -> `http://localhost:<port>`); the
`MCP server - connect a coding agent` block built its base from the serve
child's `objectstack:listening` url, i.e. the socket the child bound. Under
`OS_AUTH_URL=https://localhost:4443` with `dev -p 4001` the two disagreed,
and the block's `Connect` line is a command the reader pastes: it registered
an MCP entry against an origin discovery never advertises and a TLS proxy
never exposes.

The hint now resolves through the same function `serve` calls, with the
port the child ACTUALLY bound - so the no-variable-set case still answers
`http://localhost:<boundPort>` (auto-shifted port included) from the
resolver's own tail rather than from a second fallback. When the chain
yields no parseable origin the block is omitted rather than reprinting the
address the banner just refused to print.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8
… origin fix

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8
@github-actions github-actions Bot added the size/m label Sep 8, 2026
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/cli, touching 4 documentable anchor(s).

30 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 56ee7aef1d49d7583e0cb268dc6cbd1c34ee8fd0.

4 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 60 of 216 client-bound route-ledger rows — the other 156 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 156: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 22 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 56ee7aef1d49d7583e0cb268dc6cbd1c34ee8fd0packageMentionDocs.

Which tree this was computed on

This run read content/docs from 8652f200e599c4247aa1a160e3c43a7c27165fba — the merge of head b466fc5d90ec4621d2e5ced3bd9b004a92563389 into base 56ee7aef1d49d7583e0cb268dc6cbd1c34ee8fd0, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 8652f200e599c4247aa1a160e3c43a7c27165fba && git checkout 8652f200e599c4247aa1a160e3c43a7c27165fba
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 56ee7aef1d49d7583e0cb268dc6cbd1c34ee8fd0 b466fc5d90ec4621d2e5ced3bd9b004a92563389 && git checkout -B drift-repro 56ee7aef1d49d7583e0cb268dc6cbd1c34ee8fd0 && git merge --no-ff b466fc5d90ec4621d2e5ced3bd9b004a92563389

node scripts/docs-audit/affected-docs.mjs --json 56ee7aef1d49d7583e0cb268dc6cbd1c34ee8fd0

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 56ee7aef1d49d7583e0cb268dc6cbd1c34ee8fd0 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Sep 8, 2026

Copy link
Copy Markdown
Collaborator
VERDICT: ACCEPT
Implemented-by: `claude/issue-16734-mcp-connect-hint-origin`
Reviewed-by: `session_015QE8qk46e5CHJxyQEUjbf8`

Accepted by the domain:cli execution PM seat (#6024, R71). Re-measured at source — ⛔ a delivery report is not a reading.

Gate ① — CI

36 raw check runs → 33 after latest-per-name collapse: 30 success, 3 skipped, 0 red, 0 pending; mergeable_state=clean. ⚠️ Legacy commit statuses are not covered by the check-suite signal, so they were read separately: combined success (1 context, Vercel).

Gate ② — clause ②, verified on CONTENT, not spelling

--pair 16812 exit 0, and both declaration documents read independently (⛔ the gate's 「both carriers agree」 is about the label carriers — #16770): PR body → no, card claim 5580232384no.

⭐ The one thing that could have rebutted no is that printMcpConnectHint is newly exported. Checked directly rather than waved through:

@objectstack/cli exports : "." , "./console" , "./hook-body"     (all from dist/)
printMcpConnectHint      : commands/dev.ts ×3, its test ×4, and NOWHERE else
                           no index.ts re-exports it   (control: 7 hits in-tree, reader works)

⇒ the export reaches the sibling test file, not consumers. No published surface widens.

Acceptance — the card's three, read in the DIFF

  • Item 1 (both rows, one capture). describe('with OS_AUTH_URL set, all three hint lines and the banner row agree'), and the test drives printServerReady(...) alongside the hint so both printers land in one ordered buffer — the file's own words: "The banner row and each of the three block lines, from ONE capture." Leads with the literally reported case (dev -p 4001 under OS_AUTH_URL=https://localhost:4443), asserts localhost:4001 appears nowhere, and that the set of distinct MCP origins is exactly one. ⛔ That last assertion is what makes it a pin rather than a toContain that would pass on a mixed output.
  • Item 2 (negative control). Three unset cases: 3000, ⭐ the auto-shifted 3001 (also asserting 3000 appears nowhere), and an ephemeral 45064. This is the direction a hardcoded canonical origin destroys, and it is pinned.
  • Item 3 (reuse, not reimplement). resolveAuthBaseUrl is imported from ./serve.js — visible in the diff, with ⛔ dev declares no chain of its own written beside it. The set-but-empty OS_AUTH_URL pins in serve-auth-base-url-diagnostic.test.ts re-run unchanged (25 passed), and packages/cli/src/utils/format.ts is not in the diff — the "reference implementation, not a second thing to change" fence held.

⭐ The judgement call beyond the card — accepted, and why

When the chain yields no parseable origin (set-but-empty OS_AUTH_URL=, or a value with no scheme) the block is now omitted rather than falling back to the bound socket. The card did not ask for this, so it was examined rather than accepted:

It is entailed by item 1. If the banner refuses to print an origin, the only way for the block to agree is to print nothing — and a Connect command has no paths-only form the way the banner's paths do. Falling back to the socket would, in the function header's words, "reprint, on the same screen, the exact address the banner just refused to print" — recreating this card's defect in precisely the configuration most likely to be misconfigured. It is argued in the header, pinned by two tests, and changes no banner behaviour.

Serial — re-measured at accept time, ⛔ not carried over from dispatch

17 open PRs read, 0 empty file lists, 526 distinct files. commands/dev.ts0 other holders; utils/format.ts → 0; commands/serve.ts (newly imported from) → 0. ⭐ Positive control fired: scripts/engine-double-contract.pinned.json 3 holders, packages/objectql/src/engine.ts 2, packages/metadata-protocol/src/protocol.ts 2.

⭐ The ablation is why this acceptance is cheap

Two legs with different predicted signatures, and both matched. Reverting the origin to the listen socket reddened the three OS_AUTH_URL-set cases and both unusable-value cases while all three unset cases stayed green; separately hardcoding resolveAuthBaseUrl(3000) reddened only the shifted-port and ephemeral-port cases while item 1 stayed green. ⇒ Two independent failure modes, two distinct signatures — that asymmetry is what separates a real fix from a suite that would pass either way. On-disk mutation proven by occurrence counts and git hash-object; restore proven by blob equality and an empty git diff HEAD.

Docs-drift advisory — answered without widening the PR

The bot listed 30 pages and truncated at 15 rows. The delivery re-derived the list (34 docs, 4 anchors) instead of reading the truncated comment, and:

  • ⛔ the 4 release-owned pages were NOT edited — release notes are written centrally at release time; this PR's only input to them is its changeset. Verified from the file list: nothing under content/ is in this diff at all.
  • the other 30 were each listed through a generic anchor (the os dev command name, the two MCP path literals this diff moves but does not change, or the brand-new symbol no page can be naming) — all verified unaffected.
  • the bot's declared blind spot was hand-read: grep -rn 'objectstack:listening|listen origin|bound socket' content/zero hits; no page anywhere states which origin the hint is built from. The two pages that print a claude mcp add line show http://localhost:3000/… for a local dev server and the deployment origin for a deployed instance — which is exactly what the fixed hint now emits in each case. ⇒ the change moves the code toward those pages; before it, only the local form could ever be printed.

0 of 34 pages edited; the PR is still 3 files.

Deferred, ⛔ not filed on the delivery's word

Three observations were declined and argued (a now-readerless ListeningMessage.url, the double attribution footer, the docs bot's own precision limits). ⛔ This seat has not independently re-derived any of them, so ⛔ none is filed here — that is a separate act with its own measurement.

Landing: marked ready and routed to the merge queue. ⛔ Not merged outside the queue; ⛔ no governed surface in this diff.


Generated by Claude Code

@os-project-manager
os-project-manager marked this pull request as ready for review September 8, 2026 07:38
@os-project-manager
os-project-manager added this pull request to the merge queue Sep 8, 2026
Merged via the queue into main with commit 68fd85a Sep 8, 2026
38 checks passed
@os-project-manager
os-project-manager deleted the claude/issue-16734-mcp-connect-hint-origin branch September 8, 2026 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants